긴급입니다. bff route handler 개발 #65
Conversation
Summary of ChangesHello @yooolleee, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 인증 관련 API 호출을 위한 BFF(Backend-for-Frontend) 계층을 도입하여 애플리케이션 아키텍처에 중요한 변화를 가져옵니다. 다양한 HTTP 보안 헤더를 추가하여 애플리케이션 보안을 강화하고, 안전한 쿠키를 통한 토큰 관리를 포함한 인증 로직을 Next.js API 라우트 내에서 중앙 집중화합니다. 또한, API 클라이언트를 서버 전용으로 사용하기 위해 환경 변수 접두사를 업데이트하고 핵심 fetchApi 유틸리티를 리팩토링하여 서버 측 코드와 클라이언트 측 코드의 명확한 분리를 지향합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 BFF (Backend for Frontend) 패턴을 도입하여 인증 관련 API 라우트 핸들러를 구현하는 중요한 변경 사항을 포함하고 있습니다. 전반적으로 httpOnly 쿠키를 사용하여 토큰을 안전하게 관리하고, 서버 측에서만 API 키를 사용하도록 변경한 점은 매우 긍정적입니다. 하지만 몇 가지 중요한 문제점과 개선점이 발견되었습니다. 특히, CSP(Content Security Policy) 설정이 보안에 취약하며, 기존 인증이 필요한 API 호출들이 이번 변경으로 인해 동작하지 않게 될 수 있는 심각한 버그가 있습니다. 또한, 코드 중복과 매직 스트링 사용 등 유지보수를 저해할 수 있는 부분들도 확인되었습니다. 자세한 내용은 각 파일에 남긴 리뷰 코멘트를 참고해 주시기 바랍니다.
| "script-src 'self' 'unsafe-inline' 'unsafe-eval'", | ||
| "style-src 'self' 'unsafe-inline'", |
There was a problem hiding this comment.
Content-Security-Policy의 script-src에 'unsafe-inline'과 'unsafe-eval'을 사용하는 것은 XSS(Cross-Site Scripting) 공격에 매우 취약해질 수 있어 보안상 위험합니다.
'unsafe-inline': 인라인<script>태그나onclick같은 인라인 이벤트 핸들러를 허용합니다.'unsafe-eval':eval()함수 사용을 허용하여 임의의 문자열을 코드로 실행할 수 있게 합니다.
Next.js App Router 환경에서는 대부분 이 옵션들 없이 구현이 가능합니다. 만약 특정 라이브러리 때문에 반드시 필요하다면, 해당 내용을 문서화하고 점진적으로 제거할 계획을 세우는 것이 좋습니다. style-src의 'unsafe-inline'도 가능하면 제거하는 것을 권장합니다.
| "script-src 'self' 'unsafe-inline' 'unsafe-eval'", | |
| "style-src 'self' 'unsafe-inline'", | |
| "script-src 'self'", | |
| "style-src 'self'", |
There was a problem hiding this comment.
AI 리뷰가 정확하지만 현실적인 문제가 있습니다.
Next.js App Router는 내부적으로 인라인 스크립트를 사용하고, Storybook이나 외부 라이브러리들이 unsafe-eval을 요구하는 경우가 있어서 무턱대고 지우면 앱이 깨질 수 있어. AI가 제안한 대로 'self'만 남기면 로컬에서 바로 에러 날 가능성이 높습니다. 지금 수준으로 두되 "unsafe-inline은 Next.js 내부 동작 때문에 임시 허용한 상태이고, nonce 기반으로 개선할 수 있다는 점은 알고 있다면 되는 내용입니다.
| export function fetchApiServer(path: string, options: RequestInit = {}) { | ||
| const method = getMethod(options); | ||
| assertBodyAllowed(method, options.body); | ||
|
|
||
| const headers = new Headers(options.headers); | ||
| if (shouldSetJsonContentType(headers, options.body)) { | ||
| headers.set('Content-Type', 'application/json'); | ||
| } | ||
| if (shouldAttachDevAuthHeader(headers)) { | ||
| headers.set('Authorization', `Bearer ${DEV_ACCESS_TOKEN}`); | ||
| } | ||
|
|
||
| return fetch(buildApiUrl(path), { | ||
| ...options, | ||
| method, | ||
| headers, | ||
| }); | ||
| } |
There was a problem hiding this comment.
fetchApi를 fetchApiServer로 리팩토링하면서 인증 로직이 제거되었습니다. 이로 인해 기존에 인증이 필요했던 userApi.ts나 groups/http.ts를 통한 API 호출이 모두 인증 없이 요청되어 실패하게 됩니다. 이는 심각한 버그로 이어질 수 있습니다.
BFF 패턴에서는 API 라우트 핸들러가 쿠키에서 accessToken을 읽어 백엔드 API로 요청을 보낼 때 Authorization 헤더에 담아 전달해야 합니다. 이를 위한 인증된 요청을 처리하는 별도의 래퍼(wrapper) 함수를 만드는 것을 강력히 권장합니다.
예시:
// In a new file or here
import { cookies } from 'next/headers';
export async function fetchApiAuthenticated(path: string, options: RequestInit = {}) {
const cookieStore = await cookies();
const accessToken = cookieStore.get('accessToken')?.value;
if (!accessToken) {
// Handle unauthorized case, maybe throw an error
throw new Error('액세스 토큰이 없습니다.');
}
const headers = new Headers(options.headers);
if (!headers.has('Authorization')) {
headers.set('Authorization', `Bearer ${accessToken}`);
}
return fetchApiServer(path, {
...options,
headers,
});
}그리고 userApi.ts와 같이 인증이 필요한 API 모듈에서는 fetchApiAuthenticated를 사용하도록 수정해야 합니다.
There was a problem hiding this comment.
이것도 맞는 말이지만 저희 상황을 상황을 알고 하는 말이 아닙니다. . AI는 userApi.ts, groups/http.ts가 여전히 fetchApiServer를 직접 호출하는 걸 보고 "인증 헤더가 없다"고 경고하는 거지만 저희는 이 파일들을 프록시로 마이그레이션하는 게 목표이고, 그게 완료되면 이 파일들 자체가 없어지거나 변경되니 AI가 제안한 fetchApiAuthenticated 래퍼를 만드는 건 우리 구조에서 불필요합니다
| "style-src 'self' 'unsafe-inline'", | ||
| "img-src 'self' blob: data: https:", | ||
| "font-src 'self'", | ||
| "connect-src 'self' https://fe-project-cowokers.vercel.app", |
There was a problem hiding this comment.
connect-src에 https://fe-project-cowokers.vercel.app URL이 하드코딩되어 있습니다. 이렇게 하면 개발, 스테이징, 프로덕션 등 여러 환경을 관리하기 어려워집니다. 환경 변수에서 URL을 불러오도록 수정하는 것이 좋습니다.
| "connect-src 'self' https://fe-project-cowokers.vercel.app", | |
| "connect-src 'self' " + process.env.NEXT_PUBLIC_BACKEND_URL, |
| import { cookies } from 'next/headers'; | ||
|
|
||
| // 액세스 토큰: 짧은 만료 (1시간) | ||
| const ACCESS_TOKEN_MAX_AGE = 60 * 60; | ||
| // 리프레시 토큰: 긴 만료 (7일) | ||
| const REFRESH_TOKEN_MAX_AGE = 60 * 60 * 24 * 7; | ||
|
|
||
| export async function setAuthCookies(accessToken: string, refreshToken: string) { | ||
| const cookieStore = await cookies(); | ||
|
|
||
| cookieStore.set('accessToken', accessToken, { | ||
| httpOnly: true, | ||
| secure: process.env.NODE_ENV === 'production', | ||
| sameSite: 'lax', | ||
| path: '/', | ||
| maxAge: ACCESS_TOKEN_MAX_AGE, | ||
| }); | ||
|
|
||
| cookieStore.set('refreshToken', refreshToken, { | ||
| httpOnly: true, | ||
| secure: process.env.NODE_ENV === 'production', | ||
| sameSite: 'lax', | ||
| path: '/', | ||
| maxAge: REFRESH_TOKEN_MAX_AGE, | ||
| }); | ||
| } | ||
|
|
||
| export async function clearAuthCookies() { | ||
| const cookieStore = await cookies(); | ||
| cookieStore.delete('accessToken'); | ||
| cookieStore.delete('refreshToken'); | ||
| } |
There was a problem hiding this comment.
쿠키 이름인 'accessToken'과 'refreshToken'이 여러 곳에서 매직 스트링으로 사용되고 있습니다. 오타를 방지하고 유지보수성을 높이기 위해 상수로 정의하여 사용하는 것이 좋습니다.
import { cookies } from 'next/headers';
export const ACCESS_TOKEN_NAME = 'accessToken';
export const REFRESH_TOKEN_NAME = 'refreshToken';
// 액세스 토큰: 짧은 만료 (1시간)
const ACCESS_TOKEN_MAX_AGE = 60 * 60;
// 리프레시 토큰: 긴 만료 (7일)
const REFRESH_TOKEN_MAX_AGE = 60 * 60 * 24 * 7;
export async function setAuthCookies(accessToken: string, refreshToken: string) {
const cookieStore = await cookies();
cookieStore.set(ACCESS_TOKEN_NAME, accessToken, {
httpOnly: true,
secure: process.env.NODE_ENV === 'production',
sameSite: 'lax',
path: '/',
maxAge: ACCESS_TOKEN_MAX_AGE,
});
cookieStore.set(REFRESH_TOKEN_NAME, refreshToken, {
httpOnly: true,
secure: process.env.NODE_ENV === 'production',
sameSite: 'lax',
path: '/',
maxAge: REFRESH_TOKEN_MAX_AGE,
});
}
export async function clearAuthCookies() {
const cookieStore = await cookies();
cookieStore.delete(ACCESS_TOKEN_NAME);
cookieStore.delete(REFRESH_TOKEN_NAME);
}| import { cookies } from 'next/headers'; | ||
| import { fetchApiServer } from '@/shared/apis/fetchApi.server'; | ||
|
|
||
| const ACCESS_TOKEN_MAX_AGE = 60 * 60; |
There was a problem hiding this comment.
ACCESS_TOKEN_MAX_AGE 상수가 src/app/api/auth/_lib/cookies.ts에도 정의되어 있습니다. 중복을 피하기 위해 _lib/cookies.ts에서 export하여 import해 사용하는 것이 좋습니다. 또한, 아래 29-35줄의 쿠키 설정 로직도 중복되므로 _lib/cookies.ts에 별도의 헬퍼 함수로 추출하여 재사용하는 것을 고려해 보세요.
2 participants
Summary
간단히 마이그레이션 가이드를 첨부하겠습니다. 자세한 내용은 https://www.notion.so/30fa2ebca30b8039aefeeb37a419306b?source=copy_link
노션 링크를 확인해주세요
Issue
Scope
포함
특이사항